Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #464 +/- ##
==========================================
- Coverage 76.01% 75.89% -0.13%
==========================================
Files 152 155 +3
Lines 6204 6563 +359
==========================================
+ Hits 4716 4981 +265
- Misses 1488 1582 +94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| /// Internal implementation of NIP-77 Negentropy sync | ||
| /// This class is not part of the public API | ||
| class Nip77Internal { |
There was a problem hiding this comment.
non descriptive name. Classes can be private with _MyClass "import" possible via part
There was a problem hiding this comment.
or instead of seperating making the methods private?
frnandu
left a comment
frnandu
left a comment
There was a problem hiding this comment.
High-Value Improvements
- Verify relay origin on incoming NEG messages: processNegMsg / processNegErr trust subscriptionId only; add state.relayUrl == relayUrl check to avoid cross-relay session contamination. (packages/ndk/lib/domain_layer/usecases/nip77/nip77.dart:254, packages/ndk/lib/domain_layer/usecases/nip77/nip77.dart:290)
- NOTICE fallback detection is brittle: checking only noticeMsg.contains('negentropy') will miss many real-world unsupported-command notices. Consider broader patterns (NEG-OPEN, unsupported, unknown command) or rely primarily on NEG-ERR + capability checks. (packages/ndk/lib/domain_layer/usecases/relay_manager.dart:533)
- createInitialMessage(..., idSize) has an unused parameter; remove or use it consistently. (packages/ndk/lib/shared/nips/nip77/negentropy.dart:137)
Tests/Docs Gaps - One unit test is effectively a no-op (expect(needIds.length + haveIds.length, greaterThanOrEqualTo(0))). Replace with deterministic multi-round assertions or protocol fixture tests. (packages/ndk/test/nips/nip77_test.dart:266)
- Integration tests depend on public relays and may be flaky in CI; consider mock transport/in-memory relay tests for reliable protocol behavior. (packages/ndk/test/usecases/nip77/nip77_integration_test.dart:57)
- Docs have a duplicated section title and likely outdated fallback call (ndk.query(...) may not match current API conventions). (doc/usecases/negentropy.md:5, doc/usecases/negentropy.md:40)
| return items; | ||
| } | ||
|
|
||
| Future<List<neg.NegentropyItem>> _buildItemsFromFilter(Filter filter) async { |
There was a problem hiding this comment.
Filter mismatch in local cache query: Nip77Internal._buildItemsFromFilter only forwards authors/kinds/since/until, but NEG-OPEN sends the full filter.toMap().
This can produce false needIds/haveIds for filters using ids, tags, search, or limit.
Update loadEvents(...) call to pass the full filter surface. (packages/ndk/lib/domain_layer/usecases/nip77/nip77.dart:237)
| return Nip77Response(state); | ||
| } | ||
|
|
||
| Future<void> _startReconciliation({ |
There was a problem hiding this comment.
Race on timeout vs async startup: timeout can complete/remove session, but _startReconciliation may continue and still send NEG-OPEN because it never re-checks state.isCompleted after awaits. Add guards before send (and after major awaits). (packages/ndk/lib/domain_layer/usecases/nip77/nip77.dart:139, packages/ndk/lib/domain_layer/usecases/nip77/nip77.dart:159)
| int value = 0; | ||
| int bytesConsumed = 0; | ||
|
|
||
| while (offset + bytesConsumed < data.length) { |
There was a problem hiding this comment.
Varint decode accepts truncated payloads: decodeVarint can exit at end-of-buffer with continuation bit still set and return a partial value instead of throwing. This is a protocol correctness bug for malformed input. (packages/ndk/lib/shared/nips/nip77/negentropy.dart:57)
| } | ||
| } | ||
|
|
||
| void closeAll() { |
There was a problem hiding this comment.
Active NIP-77 sessions are not closed on Ndk.destroy(): no call path to Nip77Internal.closeAll(), so in-flight negotiation streams/futures may linger until timeout. Wire cleanup into lifecycle shutdown. (packages/ndk/lib/presentation_layer/ndk.dart:166, packages/ndk/lib/domain_layer/usecases/nip77/nip77.dart:326)
3 participants
TODO